Skip to content

GORM: Shared Mapping Registry O(M+N) Scaling (clean rebuild)#15678

Open
borinquenkid wants to merge 7 commits into
8.0.x-hibernate7from
8.0.x-hibernate7.gorm-scaling-clean
Open

GORM: Shared Mapping Registry O(M+N) Scaling (clean rebuild)#15678
borinquenkid wants to merge 7 commits into
8.0.x-hibernate7from
8.0.x-hibernate7.gorm-scaling-clean

Conversation

@borinquenkid
Copy link
Copy Markdown
Member

Description

This PR replaces #15656, which was rebuilt on a clean branch to remove a corruption commit from the history.

Problem: In multi-tenant GORM environments with many tenants (M) and domain classes (N), the previous implementation instantiated a full set of static API, instance API, and validation API objects per tenant per entity, producing O(M × N) object allocations. This caused excessive memory consumption and degraded startup performance as tenant counts scaled.

Solution: Refactor GormRegistry to use a single shared registry keyed by entity class name and qualifier (datasource/tenant), with the GORM API objects created once per entity per qualifier and reused across tenants. This reduces the allocation profile to O(M + N).

This rebuild also fixes two regression classes exposed by the new registry:

  1. Multi-tenancy resolution regressionsGormApiResolver and GormRegistry.registerEntityDatastores were routing DISCRIMINATOR/SCHEMA tenant IDs through the datasource connection lookup path, causing child datastores to be overwritten by the parent and PartitionedMultiTenancySpec.count() to NPE. Fixed by detecting multi-tenancy mode before delegating to getDatastoreForConnection, and by skipping non-DEFAULT qualifier registration when the qualifier resolves back to the parent (i.e., it's a runtime tenant ID, not a datasource name).

  2. Child datastore initialization orderHibernateDatastore (H5) and ChildHibernateDatastore (H7) were throwing ConfigurationException when getDatastoreForConnection was called for a sibling during initialization before all children were registered. Fixed to return null during the initialization phase so GormRegistry falls back gracefully and re-registers once initialization completes.

Test infrastructure: Added forkEvery = 1 to gradle/hibernate5-test-config.gradle and gradle/hibernate7-test-config.gradle. The root config uses forkEvery = 50/100 for speed, but with a shared GormRegistry singleton, TCK specs running in the same JVM before PartitionedMultiTenancySpec were clearing datastoresByQualifier["default"] and causing the NPE described above. Each test class now gets its own JVM.

Verified: H5 — 669 tests / 0 failures. H7 — 2960 tests / 0 failures.

Contributor Checklist

Issue and Scope

  • This PR is linked to an existing issue that has been acknowledged or approved by the project team. (Replaces GORM: Shared Mapping Registry O(M+N) Scaling #15656, which tracks the approved O(M+N) scaling work.)
  • This PR addresses the complete scope of the linked issue.
  • This PR contains a single, focused change.
  • This PR targets the correct branch (8.0.x-hibernate7 — major release branch; breaking API changes permitted).

Code Quality

  • I have added or updated tests that cover the changes introduced in this PR.
  • I have verified that all existing tests pass (H5: 669/0 failures, H7: 2960/0 failures).
  • My code follows the project's code style guidelines. ./gradlew codeStyle has been run and violations resolved.
  • This PR does not include unsolicited reformatting or unrelated refactoring.
  • Generative AI tooling was used in preparing this contribution with a quality model, consistent with the project's quality standards.

Licensing and Attribution

  • All contributed code is provided under the Apache License 2.0, and new source files include the appropriate Apache license header.
  • I have the necessary rights to submit this contribution and confirm it is my own original work.
  • Generative AI tooling use follows the ASF policy on generative tooling and is properly attributed.

Documentation

  • No new user-facing APIs are introduced; this is a performance/correctness fix to the internal registry.
  • The PR description explains what was changed and why.

borinquenkid and others added 4 commits May 21, 2026 18:14
Introduces shared-registry architecture to eliminate per-tenant API wrapper
duplication in multi-tenant environments with high entity/tenant cardinality.

Core changes:
- GormRegistry: normalization caches (entity keys, qualifiers), O(1) lookup paths
- GormApiResolver: simplified fallback chains, qualified API caching
- AbstractGormApiRegistry/sub-registries: normalized key/qualifier registration
- GormEnhancer: delegates API resolution through GormRegistry

Datastore integrations:
- Hibernate 7 and Hibernate 5: aligned to shared registry model
- MongoDB, Neo4j, SimpleMap, GraphQL: registry-pattern integration

Adjacent migrations:
- AsyncEntity: GormEnhancer.findStaticApi -> GormRegistry.instance.findStaticApi
- ByDatasourceDomainClassFetcher: GormEnhancer.findDatastore -> GormRegistry apiResolver
- TCK: added transaction-capable datastore support in GrailsDataTckManager

This commit excludes all collateral CodeNarc reformat changes (2,835 files
from commit 4add87e) and agent experiments, containing only the
optimization-specific module changes.

Agent collaboration note: Claude Sonnet 4.6 assisted with branch archaeology
and rebuild strategy; borinquenkid is the primary author and remains
responsible for the final changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Includes SessionResolver and ThreadLocalSessionResolver (new interfaces/classes
introduced by the O(M+N) scaling refactor), plus updates to AbstractDatastore,
AbstractMappingContext, and related core classes that the datastore modules
(SimpleMap, Hibernate 5/7) depend on at compile time.

Missed from initial clean rebuild commit.

Agent collaboration note: Claude Sonnet 4.6 assisted; borinquenkid is the
primary author and remains responsible for the final changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
During child datastore construction, GormRegistry.registerEntityDatastores
calls getDatastoreForConnection() before the parent's datastoresByConnectionSource
map is populated, throwing ConfigurationException for multi-datasource setups.

H5 anonymous child: add self-reference check so the child returns itself when
asked for its own connection name, rather than delegating to the parent map.

H7 ChildHibernateDatastore: use PARENT_HOLDER ThreadLocal to pass the parent
reference through the super() call before the parent field is assigned; also
pass the parent's datastoresByConnectionSource map to HibernateGormEnhancer
so it can resolve sibling datastores during initialize().

Fixes DataSource not found for name [secondary/schemaA] ConfigurationException
in multi-datasource and schema-per-tenant multi-tenancy test suites.

Agent collaboration note: Claude Sonnet 4.6 assisted; borinquenkid is the
primary author and remains responsible for the final changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The O(M+N) GormRegistry refactor exposed two classes of regression in
multi-tenancy and multi-datasource scenarios. This commit addresses both.

Production fixes:

GormApiResolver: Move the DISCRIMINATOR mode check before the
MultipleConnectionSourceCapableDatastore delegation so that tenant IDs are
never mistaken for datasource connection names. For the DEFAULT qualifier,
return the preferred (active-transaction) datastore directly rather than
re-routing through getDatastoreForConnection, which would return the parent
and mismatch the session factory already bound to the transaction.

GormRegistry.registerEntityDatastores: Stop overwriting child datastores with
the parent for non-DEFAULT qualifiers that resolve back to the parent. In
SCHEMA and DISCRIMINATOR mode the qualifier is a runtime tenant ID, not a
datasource name; routing it back to the parent is correct and must not clobber
the child entries added by addTenantForSchemaInternal.

GormRegistry.findTransactionManager: Fall back through the full apiResolver
when getDatastore returns null so that DISCRIMINATOR/SCHEMA tenant IDs still
resolve to a transaction manager.

HibernateDatastore (H5) / ChildHibernateDatastore (H7): Return null instead
of throwing ConfigurationException when getDatastoreForConnection is called
for a sibling that is not yet registered during initialization. GormRegistry
will re-register all entities with the correct datastores once initialization
completes. Child datastores also delegate to the parent for unrecognized
connection names so the lookup chain stays consistent.

HibernateGormInstanceApi (H7): Always resolve the template via the datastore
registry rather than caching a DEFAULT-qualifier instance, so that
preferred-datastore switching in multi-datasource transactions picks up the
correct session factory.

GrailsHibernateTransactionManager (H7): Remove debug System.err.println
statements left over from investigation.

Test infrastructure fixes:

gradle/hibernate5-test-config.gradle, gradle/hibernate7-test-config.gradle:
Set forkEvery = 1 so each test class runs in its own JVM. The root
test-config.gradle uses forkEvery = 50 (CI) / 100 (local) for speed; with a
shared GormRegistry singleton that per-test setup/teardown mutates, TCK specs
running before PartitionedMultiTenancySpec in the same JVM were clearing
datastoresByQualifier["default"], causing a NullPointerException in count()
when PartitionedMultiTenancySpec later resolved a GormPersistentEntity. forkEvery = 1
eliminates cross-class singleton contamination at the cost of extra JVM
startup overhead, which is acceptable given the test isolation requirement.

GrailsDataHibernate5TckManager: Add grailsConfig field and populate a local
ConfigObject from it in createSession(), fixing MissingPropertyException when
test specs assign grailsConfig before calling setup().

Verified: H5 669 tests / 0 failures, H7 2960 tests / 0 failures.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Local issue-tracking files have no Apache license header and are not
source artifacts. Add **/ISSUES.md to the RAT exclusion list alongside
the existing local-tasks.gradle exclusion.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@borinquenkid borinquenkid requested a review from jdaugherty May 23, 2026 13:27
…saction routing

    - GormEnhancer: Remove generic type parameters (`<D>`) from deprecated API lookup methods (`findStaticApi`, `findInstanceApi`, `findValidationApi`). This fixes a `MissingMethodException` encountered
  when older compiled code or dynamic Groovy proxies call these methods without exact signature matches.
    - TransactionalTransform: Fix `IllegalStateException: No GORM implementations configured` in multi-tenant environments. Revert logic that incorrectly passed `tenantId` as a connection source
  qualifier to `GormRegistry`, and instead fetch the tenant-specific datastore using `getTargetDatastore().getDatastoreForTenantId(tenantId)`.
    - ServiceTransformation: Ensure the stateful `$datastore` field is properly generated for generic data services to resolve injection failures in tests.
@testlens-app
Copy link
Copy Markdown

testlens-app Bot commented May 24, 2026

🔎 No tests executed 🔎

🏷️ Commit: afa32a1
▶️ Tests: 0 executed
⚪️ Checks: 2/2 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant